New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: ensure (a?.b)() has proper this #11623
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit a2d5657:
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/23268/ |
@@ -167,13 +167,19 @@ const handle = { | |||
const parentIsOptionalCall = parentPath.isOptionalCallExpression({ | |||
callee: node, | |||
}); | |||
const isParenthesizedMemberCall = | |||
parentPath.isCallExpression({ callee: node }) && | |||
node.extra?.parenthesized; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current approach does not support parserOpts.createParenthesizedExpressions
. When developing I realize that the whole transform does not take ParenthesizedExpressions
into account, I suggest we address it in a separate PR before this one gets too big to review.
optionalPath.isOptionalCallExpression() | ||
optionalPath.isOptionalCallExpression() || | ||
optionalPath.isParenthesizedExpression() || | ||
optionalPath.isTSNonNullExpression() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By doing so we also fix cases like a!!?.b
are not transformed currently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oliverdunk Should we also check for this in your PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, I'll add that now 👍
packages/babel-helper-create-class-features-plugin/src/fields.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
optionalPath.isOptionalCallExpression() | ||
optionalPath.isOptionalCallExpression() || | ||
optionalPath.isParenthesizedExpression() || | ||
optionalPath.isTSNonNullExpression() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oliverdunk Should we also check for this in your PR?
It has been out there for quite a while and the fix seems more tricky than previously proposed in #11327 and #10802.
This PR supersedes #11552 .
Todo
(a?.#m)()
parserOpts.createParenthesizedExpression